Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upstreamize VM subscriptions guide #3528

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Upstreamize VM subscriptions guide #3528

wants to merge 2 commits into from
+782 −8

Conversation

apinnick
Copy link
Contributor

@apinnick apinnick commented Dec 15, 2024 •
edited
Loading

What changes are you introducing?

Upstreamizing VM subscriptions guide.

  • Updated doc title because user story is configuring virt-who for VM subscriptions, not configuring the subscriptions themselves.
  • Updated UI procedure for creating virt-who configs for Nutanix and CNV
  • Removed troubleshooting sections that duplicated troubleshooting content in Subscription Central doc, Getting started with RHEL system registration, retaining only Satellite/Foreman procedures.

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

https://issues.redhat.com/browse/SAT-27789

Checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.13/Katello 4.15
  • Foreman 3.12/Katello 4.14 (Satellite 6.16)
  • Foreman 3.11/Katello 4.13 (orcharhino 6.11 on EL8 only)
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • We do not accept PRs for Foreman older than 3.7.

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 15, 2024 •
edited
Loading

The PR preview for f16c7f4 is available at theforeman-foreman-documentation-preview-pr-3528.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

@apinnick apinnick force-pushed the upstreamize-vm-subscription-guide branch 7 times, most recently from 4eba9e6 to 0cdf087 Compare December 16, 2024 15:11
@apinnick apinnick marked this pull request as draft December 17, 2024 11:54
@apinnick apinnick force-pushed the upstreamize-vm-subscription-guide branch 7 times, most recently from f0529d6 to 7905ee8 Compare December 18, 2024 15:42
@apinnick apinnick marked this pull request as ready for review December 18, 2024 15:46
@apinnick apinnick changed the title Upstreamize VM subscriptions guide (WIP) Upstreamize VM subscriptions guide Dec 18, 2024
@apinnick apinnick added Needs tech review Requires a review from the technical perspective Needs style review Requires a review from docs style/grammar perspective labels Dec 18, 2024
@apinnick apinnick force-pushed the upstreamize-vm-subscription-guide branch from 7905ee8 to 2c90793 Compare December 18, 2024 16:04
@apinnick
Copy link
Contributor Author

@chris1984 Please review.

maximiliankolb
maximiliankolb reviewed Jan 3, 2025
View reviewed changes
guides/common/assembly_managing-virt-who-configurations.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_managing-virt-who-configurations.adoc Show resolved Hide resolved
guides/common/modules/con_troubleshooting-virt-who.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_virtual-machine-subscriptions-overview.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_virtual-machine-subscriptions-overview.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_enabling-rhsm-debug-logging.adoc Show resolved Hide resolved
guides/common/modules/proc_removing-a-virt-who-configuration.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_editing-a-virt-who-configuration.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_editing-a-virt-who-configuration.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_editing-a-virt-who-configuration.adoc Outdated Show resolved Hide resolved
@apinnick apinnick force-pushed the upstreamize-vm-subscription-guide branch from 37f3007 to f16c7f4 Compare January 5, 2025 13:08
Lennonka
Lennonka requested changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good! I've taken a look at a couple of top-level formalities so far. Will give it another round afterwards.

guides/doc-Configuring_Virt-Who-VM-Subscriptions/master.adoc
include::common/modules/snip_guide-not-ready.adoc[]
endif::[]
ifndef::HideDocumentOnStable,foreman-deb,satellite[]
endif::[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This endif belongs at the end of the file.

guides/doc-Configuring_Virt-Who-VM-Subscriptions/master.adoc
= {ConfiguringVMSubscriptionsDocTitle}

// Render only for relevant and finished contexts
ifdef::HideDocumentOnStable,foreman-deb,satellite[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition looks strange. Is the satellite build really supposed to be "not ready"? I don't think so. The following condition should suffice because HideDocumentOnStable is defined per build context, e.g. foreman-deb:

Suggested change
ifdef::HideDocumentOnStable,foreman-deb,satellite[]
ifdef::HideDocumentOnStable[]

The same applies to the ifndef below.

guides/common/modules/snip_technology-preview.adoc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please, thank you! Very elegant too :)

guides/common/attributes-titles.adoc
@@ -6,6 +6,7 @@
:AppCentricDeploymentDocTitle: Deploying hosts by using application centric approach
:ConfiguringLoadBalancerDocTitle: Configuring {SmartProxies} with a load balancer
:ConfiguringUserAuthenticationDocTitle: Configuring authentication for {ProjectName} users
:ConfiguringVMSubscriptionsDocTitle: Configuring virt-who for virtual machine subscriptions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DocURL for Satellite downstream has to match the title in snake_case, otherwise Pantheon will refuse to build it. Please update it in attributes-satellite.adoc

guides/upstream_filename_to_satellite_link.json
@@ -2,6 +2,7 @@
"build/Administering_Project/index-satellite.html": "administering_red_hat_satellite",
"build/Configuring_Load_Balancer/index-satellite.html": "configuring_capsules_with_a_load_balancer",
"build/Configuring_User_Authentication/index-satellite.html": "configuring_authentication_for_red_hat_satellite_users",
"build/Configuring_Virtual_Machine_Subscriptions/index-satellite.html": "configuring_virtual_machine_subscriptions",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DocURL fragment goes here.

@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maximiliankolb maximiliankolb maximiliankolb left review comments

@Lennonka Lennonka Lennonka requested changes

Assignees
No one assigned
Labels
Needs style review Requires a review from docs style/grammar perspective Needs tech review Requires a review from the technical perspective Waiting on contributor Requires an action from the author
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@apinnick @maximiliankolb @Lennonka